feat: Added onReady & onClose to the API#28
Conversation
|
Test summary
Run details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
|
Could you please add tests for this? It would be great to have this tested in functional tests. |
|
@mathio yep, for sure 👍 |
| "test:functional:chrome": "yarn cypress run --browser chrome", | ||
| "prepublish": "yarn run lib", | ||
| "test:functional:firefox": "cypress run --headless --browser firefox", | ||
| "test:functional:chrome": "cypress run --headless --browser chrome", |
There was a problem hiding this comment.
If you run yarn cypress it should be always taken from the node_modules. If you run just cypress and you happen to have cypress installed globally which one is used?
The reason I like to have yarn run is that it explicitly tells you are running a custom script from package.json. For example if you had a script named version you would not be able to run it using yarn version since there is a built-in yarn command like that.
There was a problem hiding this comment.
which one is used
the local one is used, and if it's not in node_modules - then the globally installed one is used.
I personally don't like run for its explicitness; I'm a big fan of doing yarn instead of yarn install, npm i instead of npm install, etc. However, since this topic really smells like a holywar, I'll just leave these lines as I've found them :)
There was a problem hiding this comment.
the local one is used
👍
smells like a holywar
I am fine with new ideas. I too write yarn install in terminal because its short. However the package.json is written just once 😃 I am fine with changing this if the rest of the team shares your view on this.
WDYT @Gotusso @lfons @antoniofull ?
There was a problem hiding this comment.
If you ask me we should favour readability over shortness because we will read it a thousand times and yada yada yada 🤷♂️ But yeah, holy war, and seems a bit out of topic for this PR.
| - "3.6" | ||
|
|
||
| addons: | ||
| # https://stackoverflow.com/questions/57903415/travis-ci-chrome-62-instead-of-77 |
| "jsdom": "^11.6.2", | ||
| "react": "16.12.0", | ||
| "react-dom": "16.12.0", | ||
| "regenerator-runtime": "^0.13.5", |
There was a problem hiding this comment.
needed for async tests because babel is not up to date.
| "test:visual:mobile": "yarn codeceptjs --config codecept-visual.conf.js run --steps --grep @mobile", | ||
| "test:functional:debug": "yarn cypress open", | ||
| "test:functional": "yarn test:functional:chrome && yarn test:functional:firefox", | ||
| "test:functional": "yarn run test:functional:chrome && yarn run test:functional:firefox", |
Co-authored-by: Matej Lednicky <matej.lednicky@typeform.com>
# 1.0.0 (2020-05-19) ### Bug Fixes * **DIST-137:** Iframe reference ([d662c5f](d662c5f)) * Update Travis deploy job ([49673a3](49673a3)) * **DIST-139:** Fix full screen modal in absolutely positioned parent ([1a2b516](1a2b516)) ### Features * Added onReady & onClose to the API ([#28](#28)) ([1a6bc27](1a6bc27)) * **DIST-137:** Use current href value when opening the popup ([ab48a22](ab48a22)) * **DIST-39:** Initial commit ([53c8edd](53c8edd))
# [0.18.0](v0.17.0...v0.18.0) (2020-05-21) ### Features * Added onReady & onClose to the API ([#28](#28)) ([1a6bc27](1a6bc27))
# [0.18.0](v0.17.0...v0.18.0) (2020-05-21) ### Features * Added onReady & onClose to the API ([#28](#28)) ([1a6bc27](1a6bc27))
|
🎉 This PR is included in version 0.18.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
4 participants
No description provided.